Skip to content

Gazebo plugin advertise add group after simulation starts - #104

Merged
iamtesch merged 4 commits into
HebiRobotics:developfrom
josephcoombe:gazebo_plugin_advertise_add_group_after_sim_start
Mar 6, 2019
Merged

Gazebo plugin advertise add group after simulation starts#104
iamtesch merged 4 commits into
HebiRobotics:developfrom
josephcoombe:gazebo_plugin_advertise_add_group_after_sim_start

Conversation

@josephcoombe

Copy link
Copy Markdown
Contributor

Simple patch for #103

private:

physics::ModelPtr model;
bool first_sim_iteration;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super minor (and there are a lot of more significant style issues in the code right now), but I'd probably change this to
bool first_sim_iteration{true}; here. That way you don't have to set the variable in the load function below, and there is never a period where is is undefined.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Will change.

@iamtesch iamtesch left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, looks good -- I just made one small suggestion. Thanks!

@josephcoombe

Copy link
Copy Markdown
Contributor Author

Changed with iamtesch's suggestion

@iamtesch iamtesch left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for not getting this in much sooner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants